Adding handling for errors parsing grant round funds #621
Conversation
|
@phutchins it looks like the tests have failed for linting |
|
Replaced undefined return with empty object instead which appears to be passing linting. |
| } catch { | ||
| // If parsing funds for the round fails, ignore the round | ||
| console.log('Failed to parse funds for round'); | ||
| return {}; |
There was a problem hiding this comment.
We normally return a grant round which has fields that are expected to be defined based on the type definitions, which aren't necessarily reflected by the LocalForage abstractions—are you sure returning an empty grantRound object like this is safe and won't cause other issues?
There was a problem hiding this comment.
@mds1 what issues do you anticipate? I tested it out and seems to be fixing the error (hanging load) we're seeing, but there could be some unforeseen issues
There was a problem hiding this comment.
There could also be a more gracious way of fixing the error
There was a problem hiding this comment.
@mds1 there are a couple of places we use this method like where we update grant rounds.
We should set a standard way of doing this (if we have not already) throughout our codebase. I.E. if you request an object from a method and it does not exist, fails to retrieve, or has errors, we should consistently return the same thing across the board. That way anywhere we are implementing that method, we can handle the results consistently.
There was a problem hiding this comment.
Looks good and fixes the hanging loading spinner. I was curious why you needed to return. I'm assuming it's to exit out of the function if error is caught. Returning an empty object is probably okay for that.
nit: maybe use console.error instead of console.log?
|
Going to move forward with merging this and we can add an issue for ensuring that we handle errors processing grant rounds consistently. |
- @dgrants/[email protected] - @dgrants/[email protected] - @dgrants/[email protected] - @dgrants/[email protected]
Closes #616